-
Notifications
You must be signed in to change notification settings - Fork 4.2k
docs: create schema for enrollment api #37846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
docs: create schema for enrollment api #37846
Conversation
|
Thanks for the pull request, @wgu-jesse-stewart! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive documentation for the Instructor Dashboard Enrollment API v2, establishing the design foundation for migrating enrollment management functionality from legacy endpoints to RESTful patterns that support MFE development.
Key changes:
- Documents RESTful API design with sync/async execution models based on operation scope
- Specifies pagination, optional fields, authentication, error handling, and email notification patterns
- Establishes OpenAPI specification as implementation reference (to be deleted after deployment)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lms/djangoapps/instructor/docs/references/instructor-v2-enrollment-api-spec.yaml |
Complete OpenAPI 2.0 specification defining enrollment endpoints, request/response schemas, authentication, and error handling |
lms/djangoapps/instructor/docs/decisions/0003-instructor-enrollment-api-spec.rst |
ADR documenting design decisions including RESTful patterns, sync/async execution model, pagination strategy, and alternatives considered |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lms/djangoapps/instructor/docs/references/instructor-v2-enrollment-api-spec.yaml
Outdated
Show resolved
Hide resolved
lms/djangoapps/instructor/docs/decisions/0003-instructor-enrollment-api-spec.rst
Outdated
Show resolved
Hide resolved
brianjbuck-wgu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read through this and didn't see anything glaring.
lms/djangoapps/instructor/docs/decisions/0003-instructor-enrollment-api-spec.rst
Outdated
Show resolved
Hide resolved
lms/djangoapps/instructor/docs/decisions/0003-instructor-enrollment-api-spec.rst
Outdated
Show resolved
Hide resolved
|
This is great, thanks for pulling it together! |
feanil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API design generally looks good. Is there a reason this needs to be an Instructor specific API? We currently have a v1 of the enrollment api: https://master.openedx.io/api-docs/#/enrollment and I'm wondering if this would make sense as just the V2 of the instructor API. It could be used by the new instructor dash but also be a generally available enrollment API for other use cases. What do you think @wgu-jesse-stewart
Great point! I agree this should not be tightly scoped to the Instructor Dashboard. The original framing came from the Instructor Dashboard modernization work, but the intent is for this to serve as the canonical v2 enrollment API that the new instructor MFE consumes, rather than a dashboard-specific surface. I’ll update the ADR and OpenAPI spec to remove instructor-specific naming where possible and position this explicitly as the successor to the existing v1 enrollment API, with the same endpoints usable across other enrollment management use cases. Thanks for calling this out. |
a4f8f26 to
6d63c56
Compare
6d63c56 to
f004adf
Compare
lms/djangoapps/instructor/docs/references/enrollment-v2-api-spec.yaml
Outdated
Show resolved
Hide resolved
| type: boolean | ||
| description: Send email notification to learners | ||
| default: false | ||
| reason: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this? on Frontend mocks we don't have a way to set it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diana-villalvazo-wgu Yes, we need this in v2. The auto_enroll parameter is part of the existing v1 API (students_update_enrollment at lms/djangoapps/instructor/views/api.py:739) and is used to auto-enroll users when they register (for non-registered users with CourseEnrollmentAllowed records). Including it in v2 maintains backward compatibility and avoids regressions when migrating from v1. While the instructor dashboard frontend may not expose this setting, the v2 API is intended to be a general-purpose enrollment API used by other services and MFEs beyond just the instructor dashboard, so we should preserve all existing v1 functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was asking about reason attribute, auto enroll is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diana-villalvazo-wgu The reason parameter is also part of the existing v1 API (students_update_enrollment at lms/djangoapps/instructor/views/api.py:741). It's used for audit trail purposes - the reason is recorded in ManualEnrollmentAudit records (line 858-859) to track why an enrollment change was made.
1d22a68 to
528b65d
Compare
lms/djangoapps/instructor/docs/decisions/0004-instructor-enrollment-api-spec.rst
Show resolved
Hide resolved
c262b91 to
3c491c2
Compare
Replace first_name/last_name with full_name in enrollment response examples and schema. Add beta_tester field to response examples. Co-authored-by: Sarina Canelake <[email protected]>
…GU-Open-edX/edx-platform into 37813/v2-enrollment-api-schema
…-enrollment-api-spec.rst
Head branch was pushed to by a user without write access
3c491c2 to
7d621d6
Compare
Issue: 37814
Description
docs: Add ADR and OpenAPI specification for Instructor Enrollment API This PR adds an architectural decision record (ADR) and OpenAPI specification for the Instructor Dashboard Enrollment API endpoints. These documentation files provide the design rationale and technical contract for the v2 enrollment API implementation, following Open edX REST API Conventions.
Changes
Supporting information
Related to the Instructor Dashboard modernization effort. These documents establish the foundation for migrating enrollment management functionality from legacy endpoints (lms/djangoapps/instructor/enrollment.py) to RESTful patterns supporting MFE development. The design follows Open edX REST API Conventions including: